feat: Add new includeIgnoreFile() to config-helpers#430
feat: Add new includeIgnoreFile() to config-helpers#430DMartens merged 37 commits intoeslint:mainfrom
includeIgnoreFile() to config-helpers#430Conversation
kirkwaiblinger
changed the title
includeIgnoreFIle() to config-helpersincludeIgnoreFile() to config-helpers
| }); | ||
|
|
||
| it("should return an object with an `ignores` property", () => { | ||
| const ignoreFilePath = fileURLToPath( |
There was a problem hiding this comment.
I think we should simplify this and in other spots by using import.meta.resolve (requires Node v18+)
| const ignoreFilePath = fileURLToPath( | |
| const ignoreFilePath = import.meta.resolve("../tests/fixtures/ignore-files/gitignore1.txt") |
There was a problem hiding this comment.
Huh... I never knew about import.meta.resolve. But in any case it looks like that still gets us a URL, which would need to be converted to a path with fileURLToPath(). So I'm not sure that that gains us anything? Unless I'm misunderstanding.
At the end of the day, this is really just the workaround for not having access to import.meta.dirname, which the linter complains about (due to supporting older versions of node)
Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
mdjermanovic
left a comment
mdjermanovic
left a comment
There was a problem hiding this comment.
LGTM, thanks! Leaving open for others to verify their review comments.
lumirlumir
left a comment
lumirlumir
left a comment
There was a problem hiding this comment.
LGTM. Thanks for your time and effort on this PR!
Would like @mdjermanovic to verify the changes before merging.
Co-authored-by: 루밀LuMir <rpfos@naver.com>
| ]); | ||
| }); | ||
|
|
||
| // convert above to for ... of loop |
There was a problem hiding this comment.
I think this is an unresolved TODO comment related to line 61 which uses Array#forEach to create a parameterised test case.
There was a problem hiding this comment.
Gah! that was a prompt to copilot that I forgot to remove, yep 🫣. Thanks for pointing that out.
|
Changes LGTM, thank you for the continued work on this. |
github-project-automation
Bot
moved this from Second Review Needed
to Complete
in Triage
Prerequisites checklist
AI acknowledgment
What is the purpose of this pull request?
fixes #329
What changes did you make? (Give an overview)
Deprecate
includeIgnoreFile(),convertIgnorePatternToMinimatch()in @eslint/compat. Add and export newincludeIgnoreFile()(documented),convertIgnorePatternToMinimatch()(undocumented) in @eslint/config-helpers.The new
convertIgnorePatternToMinimatch()is unaltered.The new
includeIgnoreFile():gitignoreResolutionoption which can befalse(default; interprets patterns relative to config file) ortrue(interprets patterns relative to ignore file).Related Issues
#329
Is there anything you'd like reviewers to focus on?
There is a companion PR in eslint repo: eslint/eslint#20735